Skip to content

Fix/root detection #185

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 7 commits into
base: development
Choose a base branch
from
Open

Fix/root detection #185

wants to merge 7 commits into from

Conversation

ubidefeo
Copy link
Collaborator

@ubidefeo ubidefeo commented Apr 12, 2025

This PR introduces detection of board's default root path, which may be / or /flash depending on ports.
It seems that it will eventually standardise to /flash following the introduction of ROMFS.
A typical scenario will be as described in this comment related to MAPFS (later called ROMFS)
micropython/micropython#8381 (comment)

/flash
/rom
/sd

With / not being directly writable unless wanted by someone producing their own custom build, but I'd rather be future-proof with this double-handling based on the presence of /flash in sys.path.

With this PR the creation of a new program will happen in /flash for boards which are already mounting the main file-system there and default to / for other platforms.
Eventually all platforms should follow custom even if a board will never use an SD card or have ROMFS

Signed-off-by: ubi de feo <me@ubidefeo.com>
@ubidefeo ubidefeo requested review from sebromero and Copilot April 12, 2025 20:45
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 27 out of 29 changed files in this pull request and generated 2 comments.

Files not reviewed (2)
  • package.json: Language not supported
  • ui/arduino/main.css: Language not supported
Comments suppressed due to low confidence (1)

ui/arduino/store.js:1633

  • There is a potential memory leak here because every new tab registers an onChange listener without removing it when the tab is closed. Consider implementing a cleanup to remove the listener upon tab closure.
newFile.editor.onChange = function() { newFile.hasChanges = true; emitter.emit('render') }

let tooltipEl = html``
if (tooltip) {
tooltipEl = html`<div class="tooltip">${tooltip}</div>`
}
tooltipEl = html``
Copy link
Preview

Copilot AI Apr 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resetting tooltipEl to an empty value immediately after conditionally setting it discards any provided tooltip content. Remove or adjust this line so that the tooltip renders as intended.

Suggested change
tooltipEl = html``

Copilot is powered by AI, so mistakes are possible. Review output carefully before use.

@ubidefeo ubidefeo changed the base branch from main to development April 12, 2025 20:53
@ubidefeo ubidefeo marked this pull request as draft April 12, 2025 20:53
@ubidefeo
Copy link
Collaborator Author

Converted to Draft since creation of files still somehow defaults to / hence failing on boards with root mount point in /flash

Signed-off-by: ubi de feo <me@ubidefeo.com>
Signed-off-by: ubi de feo <me@ubidefeo.com>
@ubidefeo ubidefeo marked this pull request as ready for review April 13, 2025 10:37
Signed-off-by: ubi de feo <me@ubidefeo.com>
Copy link
Collaborator

@sebromero sebromero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see comments

@@ -30,3 +40,5 @@ def delete_folder(path):
if file['type'] == 'folder':
os.rmdir(file['path'])
os.rmdir(path)

os.chdir(get_root())
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should not be necessary to change to the root. The system should be mounted by default there. Unless someone is using some code that changes that. Then again, the board does a soft reset when connecting. Can you elaborate?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless someone is using some code that changes that

exactly this

Signed-off-by: ubi de feo <me@ubidefeo.com>
@ubidefeo ubidefeo marked this pull request as draft April 15, 2025 09:09
@ubidefeo ubidefeo requested a review from sebromero April 15, 2025 10:57
@ubidefeo ubidefeo marked this pull request as ready for review April 18, 2025 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants